Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle race condition #244

Merged
merged 3 commits into from Dec 11, 2014
Merged

Handle race condition #244

merged 3 commits into from Dec 11, 2014

Conversation

drbyte
Copy link
Member

@drbyte drbyte commented Dec 10, 2014

Rewrite some core queries to handle race conditions more effectively

zen_db_input($val) . "')";
$result = $db->Execute($sql);
}
$sql = "insert ON DUPLICATE KEY UPDATE " . TABLE_SESSIONS . " (sesskey, expiry, value)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this probably works, the syntax does not match the MySQL documentation. To be on the safe side I would recommend changing the syntax to match. Perhaps something similar to the following?

$sql = "INSERT " . TABLE_SESSIONS . " (sesskey, expiry, value) VALUES (:sesskey, :expiry, :value) ON DUPLICATE KEY UPDATE sesskey=:sesskey, expiry=:expiry, value=:value"
$sql = $db->bindVars($sql, ':sesskey', $key, 'string');
$sql = $db->bindVars($sql, ':expiry', $expiry, 'integer');
$sql = $db->bindVars($sql, ':value', $val, 'string');
$result = $db->Execute($sql);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

@lhungil
Copy link

lhungil commented Dec 11, 2014

👍

zen_db_input($val) . "')";
$result = $db->Execute($sql);
}
$sql = "insert into " . TABLE_SESSIONS . " (sesskey, expiry, `value`)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the extra backticks around value are simply to provide specificity since value is a MySQL keyword too.

@drbyte drbyte added this to the v1.6.0 milestone Dec 11, 2014
@zcwilt
Copy link
Member

zcwilt commented Dec 11, 2014

👍

1 similar comment
@ajeh
Copy link
Member

ajeh commented Dec 11, 2014

👍

zcwilt added a commit that referenced this pull request Dec 11, 2014
@zcwilt zcwilt merged commit b7abd06 into zencart:v160 Dec 11, 2014
@drbyte drbyte deleted the race-cond branch December 11, 2014 18:46
@drbyte drbyte restored the race-cond branch December 12, 2014 18:57
@drbyte
Copy link
Member Author

drbyte commented Dec 12, 2014

Note: additional code change in #245

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants